951770 - Reject invalid WebM/Opus files

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Ralph Giles (:rillian)]

In constructing tests for VP9 playback, we discovered Firefox will play badly muxed Opus tracks in webm. We should reject them instead.

In particular, we should reject files whose CodecDelay track property doesn't match the value from the OpusHeader CodecPrivate.

Comment 1

[Jan Gerber]

Attachment: 0001-Bug-951770-Reject-invalid-WebM-Opus-files-r-rillian.patch

Comment 2

[Ralph Giles (:rillian)]

Comment on attachment 8350500 [details] [diff] [review]
0001-Bug-951770-Reject-invalid-WebM-Opus-files-r-rillian.patch

Review of attachment 8350500 [details] [diff] [review]:
-----------------------------------------------------------------

Can you add a test file for this as well?

Comment 3

[Ralph Giles (:rillian)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4cddbc0d5183

Better to get this in even without a test.

Comment 4

[Ralph Giles (:rillian)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b4978efcdad9

The code generates a signed-compare warning because mCodecDelay is an unsigned long long, but CheckedInt64::value() returns an signed int64_t. Casting this warning away is safe.

I built this locally, but without -Werror, so I didn't notice the problem until tbpl failed to build.

Comment 5

[Ralph Giles (:rillian)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ed795ed2bc38

Not my night.

Comment 6

[Phil Ringnalda (:philor)]

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/2b6516c8713e for timeouts in test_playback.html.

Comment 7

[Ralph Giles (:rillian)]

Attachment: reject invalid Opus pre-skip in WebM v2

Roll up warning and semicolon fixes from the previous landing attempt.

Tests failed because detodos.webm had exactly the corrupt header we now reject.

- move detodos.webm to invalid-preskip.webm
- make a new detodos.webm.

Also changed the debug message to use PR_LOG_WARNING and mention the file is invalid.

Comment 8

[Ralph Giles (:rillian)]

https://tbpl.mozilla.org/?tree=Try&rev=7b096ba865fe

Comment 9

[Ralph Giles (:rillian)]

Thanks, Chris. Here we go again.

https://hg.mozilla.org/integration/mozilla-inbound/rev/06e99d1896ce

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/06e99d1896ce

Comment 11

[Ralph Giles (:rillian)]

Comment on attachment 8369293 [details] [diff] [review]
reject invalid Opus pre-skip in WebM v2

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 

Feature Bug 938686.

User impact if declined:

There's some buggy encoder software generating invalid files. We want user agents to reject these as soon as possible to ensure fixed encoders get deployed.

Testing completed (on m-c, etc.):

Landed on m-c with tests, verified manually against wild files.

Risk to taking this patch (and alternatives if risky):

Risk is low. This is a new media format and use by content developers is still in the exploratory stage. The larger risk is that some bug in the patch affects playback of non-Opus WebM files, but we have tests.

String or IDL/UUID changes made by this patch:

none.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/dd51a2464396